Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#215 [refactor] 지원서 상세보기 api service 분리 #216

Merged
merged 7 commits into from
Feb 7, 2024

Conversation

pkl0912
Copy link
Contributor

@pkl0912 pkl0912 commented Jan 30, 2024

관련 이슈번호

해결하는 데 얼마나 걸렸나요? (예상 작업 시간 / 실제 작업 시간)

해결하려는 문제가 무엇인가요?

  • 지원서 상세보기 service 분리

어떻게 해결했나요?

  • modelRetrieve service 사용,
  • getIsSend 에서 offer 가 있는지 없는지 확인할 때 offerRetrieve 를 쓸까말까 고민했는데...
    offerRetrieve 를 쓰면 applicationRetirieve 랑 순환참조가 나서 안되더라고요...
    그래서 일단 getIsSend를 applicationRetrieve 에 넣어놨는데
    좋은방법 있으면 공유 부탁드립니다!

@pkl0912 pkl0912 added this to the 1차 리팩토링 기간 milestone Jan 30, 2024
@pkl0912 pkl0912 requested review from hellozo0 and KWY0218 January 30, 2024 16:20
@pkl0912 pkl0912 self-assigned this Jan 30, 2024
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2

좋은방법 있으면 공유 부탁드립니다!

Response로 보낼 값을 컨트롤러 단에서 조립하는 것은 어떨까요?
현재 최종적으로 클라이언트에게 반환할 정보들을 보면 application, model, offerIsSend 정보로 보입니다.
DesignerController에서
ModelRetrieveService를 통해 ModelDto를
ApplicationRetrieveService를 통해 ApplicationDto를
OfferRetrieveService를 통해 isOffer정보를
반환받은 후
컨트롤러 단에서 해당 Dto들을 조합해서 Response로 변환한 다음에 클라이언트에게 반환하는 것은 어떨까요?

이렇게 하면 컨트롤러 단에 서비스 의존이 뒤죽박죽이 되겠지만...
서비스 단의 코드는 더욱 간결해질 것이고, 서비스간의 의존성은 줄어들 것이라고 생각이 듭니다..!

어떻게 생각하시나요???

@@ -87,7 +87,7 @@ public SuccessNonDataResponse offerCreateRequest(
public SuccessResponse<ApplicationDetailInfoResponse> getApplicationDetailInfo(
@Parameter(hidden = true) @UserId Long userId,
@PathVariable(value = "applicationId") Long applicationId) {
return SuccessResponse.success(SuccessCode.MODEL_APPLICATION_DETAil_INFO_SUCCESS, designerService.getApplicationDetail(userId, applicationId));
return SuccessResponse.success(SuccessCode.MODEL_APPLICATION_DETAil_INFO_SUCCESS, hairModelApplicationRetrieveService.getApplicationDetail(userId, applicationId));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3
여기서도 userId를 designerId로 수정하면 좋을 것 같습니다!

Comment on lines 85 to 87

List<PreferRegion> preferRegions = preferRegionJpaRepository.findAllByModelId(modelId);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2
Offer 서비스에서 해당 api를 위해 preferRegion을 참조하는 것 보다
modelRetrieve를 통해서 모델 정보를 가져올 때 preferRegion 정보도 같이 가져오는 것이 어떨까요?

@KWY0218 KWY0218 removed this from the 1차 리팩토링 기간 milestone Jan 31, 2024
@KWY0218
Copy link
Member

KWY0218 commented Jan 31, 2024

@pkl0912
추가로 마일스톤은 이슈에만!!! 지정하면 좋을 것 같습니다!!

Copy link
Member

@hellozo0 hellozo0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good.....파일 변동이 많아서 한번 싹 정리되고 다시 올께요... 굳굳...

Comment on lines 101 to 123
ApplicationDto applicationDto = hairModelApplicationRetrieveService.getApplicationDetailInfo(applicationId);
ApplicationModelInfoDto modelInfoDto = modelRetrieveService.getApplicationModelInfo(applicationId);
ApplicationInfoResponse applicationInfoResponse = new ApplicationInfoResponse(
applicationId,
applicationDto.modelImgUrl(),
applicationDto.hairLength(),
applicationDto.preferHairStyleList(),
applicationDto.recordResponseList(),
applicationDto.hairDetail(),
hairServiceOfferRetrieveService.getIsSendStatus(applicationId, designerId)
);

ModelInfoResponse modelInfoResponse = new ModelInfoResponse(
modelInfoDto.modelId(),
modelInfoDto.name(),
modelInfoDto.age(),
modelInfoDto.gender(),
modelInfoDto.regionList(),
applicationDto.instgramId()
);

ApplicationDetailInfoResponse applicationDetailInfoResponse = new ApplicationDetailInfoResponse(applicationInfoResponse,modelInfoResponse);
return SuccessResponse.success(SuccessCode.MODEL_APPLICATION_DETAil_INFO_SUCCESS, applicationDetailInfoResponse);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2 :
이 친구들도 DesignerController가 아닌 ApplicationController를 만들어서 그곳에 이동해야할것 같습니다~!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hellozo0
사실.. 메서드 별 Tag가 되는 줄 모르고 컨트롤러를 분리하면 swagger가 분리되기 때문에 controller 분리를 하지 말기로 말하긴 했습니다..!
그래서 이번 스프린트엔 컨트롤러 분리는 선택으로 가져갈까 합니다..!
어떻게 생각하시나요..?!

Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하시느라 고생하셨습니다!
코맨트 확인해주세요!

Comment on lines 42 to 45
public Boolean getIsSendStatus(final Long applicationId, final Long userId) {
Optional<HairServiceOffer> offer = hairServiceOfferJpaRepository.findByHairModelApplicationIdAndDesignerId(applicationId, userId);
return offer.isPresent();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2
객체 타입인 Boolean 말고 원시 타입인 boolean을 사용해주세요!

Comment on lines 82 to 86
List<PreferRegion> preferRegions = preferRegionJpaRepository.findAllByModelId(modelId);

List<String> regionList = preferRegions.stream().map(preferregion -> {
return preferregion.getRegion().getName();
}).collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2
해당 구문은 사용하지 않는 구문인 것 같습니다!
사용하지 않는 구문은 제거해주세요!

또한, 해당 구문을 지우면 PreferRegionRepository도 사용하지 않게될텐데 해당 변수와 import도 같이 제거해주세요!

pkl0912 added 2 commits February 6, 2024 17:13
…into refactor/#215

# Conflicts:
#	src/main/java/com/moddy/server/service/application/HairModelApplicationRetrieveService.java
#	src/main/java/com/moddy/server/service/offer/HairServiceOfferRetrieveService.java
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다 ~ !!!

Comment on lines 54 to 55
private final ModelRetrieveService modelRetrieveService;
private final HairServiceOfferRetrieveService hairServiceOfferRetrieveService;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2
사용하지 않는 변수는 제거해주세요!

@pkl0912 pkl0912 merged commit edbd217 into develop Feb 7, 2024
1 check passed
@pkl0912 pkl0912 deleted the refactor/#215 branch February 7, 2024 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] 지원서 상세보기 service 분리
3 participants